Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Spidercoordinator: It able to get CIDR from kubeadm-config #3062

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Spidercoordinator: It able to get CIDR from kubeadm-config #3062

merged 1 commit into from
Jan 24, 2024

Conversation

cyclinder
Copy link
Collaborator

If the kube-controller-manager Pod is running as systemd process rather than Pod, In this case, We can't get the CIDR from the KCM Pod. We can get the CIDR from the kubeadm-config configMap.

Thanks for contributing!

What type of PR is this?

  • release/bug

What this PR does / why we need it:

Kube-controller-pod is running as systemd process rather than a pod, We can't get the cluster CIDR in this case.

  1. We get the clusterCIDR from kubeadm-config ConfigMap first.
  2. If kubeadm-config ConfigMap does exist in cluster, we can get it from kube-controller-manager.

Which issue(s) this PR fixes:

Fixes #3054

Special notes for your reviewer:

@cyclinder cyclinder added release/bug cherrypick-release-v0.8 Cherry-pick the PR to branch release-v0.8. cherrypick-release-v0.9 labels Jan 5, 2024
@cyclinder cyclinder mentioned this pull request Jan 5, 2024
Closed
@codecov Codecov
Copy link

codecov bot commented Jan 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e247e6d) 81.05% compared to head (712f10c) 81.10%.
Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3062      +/-   ##
==========================================
+ Coverage   81.05%   81.10%   +0.05%     
==========================================
  Files          49       49              
  Lines        5351     5351              
==========================================
+ Hits         4337     4340       +3     
+ Misses        856      854       -2     
+ Partials      158      157       -1
Flag Coverage Δ
unittests 81.10% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 4 times, most recently from 87b44fa to 5533503 Compare January 5, 2024 08:01
@weizhoublue
Copy link
Collaborator

there are so many cases already and it is import . Is it possible to design relevant E2E cases to long term test ?

and it could finish #2905 as well recently in another PR

@cyclinder
Copy link
Collaborator Author

#2927 is some E2E cases for spidercoordiantor, we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

@weizhoublue
Copy link
Collaborator

#2927 is some E2E cases for spidercoordiantor, we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

do you already have two cases to get CIDR from configmap and pod yaml separately ?

@cyclinder
Copy link
Collaborator Author

we can also add a case for these changes: delete the kubeadm-config configMap and see if works, but we can't delete kube-controller-manager pod, it's so dangerous. It can be finished in #2905.

Not yet. It can be finished in this PR Or #2905

@weizhoublue
Copy link
Collaborator

I suggest to finish the E2E together for verification

@weizhoublue
Copy link
Collaborator

I suggest to finish the E2E together for verification

is there any update

@cyclinder
Copy link
Collaborator Author

is there any update

Still working on this

@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from 5533503 to a4b5bfd Compare January 17, 2024 10:32
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 8 times, most recently from 54a2fcf to a462243 Compare January 18, 2024 03:27
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from a462243 to df9ce4a Compare January 18, 2024 05:52
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 3 times, most recently from cfb3856 to ef2f046 Compare January 18, 2024 07:28
ty-dc
ty-dc previously approved these changes Jan 18, 2024
View reviewed changes
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch 5 times, most recently from e907c33 to 83be433 Compare January 22, 2024 13:15
@cyclinder
Spidercoordinator: It able to get CIDR from kubeadm-config
712f10c 
If the kube-controller-manager Pod is running as systemd precess rather than Pod, In this case, We can't get the CIDR from the KCM Pod. We can get the CIDR from the kubeadm-config configMap.

Signed-off-by: cyclinder <[email protected]>
@cyclinder cyclinder force-pushed the spidercoorinator/sync_cluster_cidr branch from 83be433 to 712f10c Compare January 22, 2024 14:31
@cyclinder
Copy link
Collaborator Author

/cc @weizhoublue

@ty-dc ty-dc merged commit 421bfb0 into spidernet-io:main Jan 24, 2024
43 checks passed
This was referenced Jan 24, 2024
Closed
Closed
@ty-dc ty-dc mentioned this pull request Jan 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weizhoublue weizhoublue weizhoublue approved these changes

@ty-dc ty-dc ty-dc approved these changes

@Icarus9913 Icarus9913 Awaiting requested review from Icarus9913

@windsonsea windsonsea Awaiting requested review from windsonsea windsonsea is a code owner

@bzsuni bzsuni Awaiting requested review from bzsuni

Assignees
No one assigned
Labels
cherrypick-release-v0.8 Cherry-pick the PR to branch release-v0.8. cherrypick-release-v0.9 release/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

spiderpool does not support binary installation of k8s?
3 participants
@cyclinder @weizhoublue @ty-dc